Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove most of crypto and move out of search path #23

Merged
merged 8 commits into from
Jul 4, 2024

Conversation

jagerman
Copy link
Member

We only use keccak out of cncrypto, so this removes all the other junk, and minimally modifies the keccak implementation to build in ethyl without needing other files.

This also puts it under a src/ethyl-keccak subdirectory so that when we are building with cncrypto from oxen-core, we won't find the local version in our include path and instead get the appropriate headers via oxen-core when building with oxen-core's cncrypto.

(This is on top of #22 to avoid conflicts: that PR made some small updates to some of the files being removed here).

Previous iteration of the API favoured returning hex from the interface
and working with a bunch of hex strings. What this leads to is

- Extra unneeded computation. Pretty much all libraries work with
binary data which meant a round-trip hex conversion everytime we produced data.

- String validation code is present everywhere. We have to ensure that
all the characters are hex and if they start with '0x' or even '0X' we
trim those out. This is a lot of extra work that is avoided by working
in binary.

I've relegated most of the dynamic containers to those that need
dynanism (like serializing a transaction to RLP since we have variable
data payloads). Everything else, especially hashes and cryptography are
now represented with fixed byte buffers.

All APIs generate binary data first, _then_ there're helper functions
that convert the binary to hex if hex is needed (e.g. serialising to JSON).
This ensures that we trim leading zeros off the string such that
the primitive checks for converting into a U64 that do a bounds check
against the length of the string vs the largest possible number to
be stored work more permissively with large 0 padded strings.
@jagerman jagerman marked this pull request as draft June 12, 2024 22:16
We only use keccak out of cncrypto, so this removes all the other junk,
and minimally modifies the keccak implementation to build in ethyl
without needing other files (and updates the keccak signature to match
oxen-core's, which changed the last argument from int to size_t).

This also puts it under a src/ethyl-keccak subdirectory so that when we
are building with cncrypto from oxen-core, we won't find the local
version in our include path and instead get the appropriate headers via
oxen-core when building with oxen-core's cncrypto.
@jagerman jagerman marked this pull request as ready for review June 20, 2024 23:57
@Doy-lee Doy-lee merged commit 1fa7623 into oxen-io:master Jul 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants